Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

2nd PoC for Typed GQL Queries Design Doc #2389

Closed
wants to merge 12 commits into from

Conversation

fishi0x01
Copy link
Contributor

@fishi0x01 fishi0x01 commented May 10, 2022

⚠️ Show-case only! Needs some adjustments before actual merge

This is a PoC to leverage a simple custom code generator for gql queries to python

Background

Design Doc

GraphQL data is currently used in un-typed nested dictionaries. Adding type safety will increase maintainability of the code base. Non-compliant schema changes (some integrations are using a schema that you might not be aware of) are more likely to raise errors during development.

We use a similar approach for our go integrations.

Reviewer Notes

Query examples and their corresponding generated code can be found in reconcile/gql_queries.

The module code_generator is a proof the concept, i.e., it is not feature complete with everything GQL has to offer. However, it covers our most common use-cases, like standard queries and inline fragments. If we decide to go forward with a custom code generator approach, then it requires a bit more work to adhere to best practices.

The module reconcile.gql_queries contains the query definitions .gql files and the corresponding generated .py files. Those .py files are then consumed in reconcile.

Implementation Details

A query is defined in a .gql file. The content of a .gql file is a valid query that could be used with curl against a GQL backend.
All .gql files are placed under the gql_queries directory.

Running make generate-queries will:

  1. Run an introspection query against a GQL backend, i.e., fetch the schema
  2. Read all .gql files under reconcile/gql_queries
  3. For each .gql file, generate the AST and match it with the types from the schema, i.e., generate a typed AST
  4. Generate a corresponding .py file for each .gql file and place it in the same (sub)directory

In the end the reconcile/gql_queries directory could look like this:

reconcile/gql_queries/
├── __init__.py
└── saas_files
    ├── __init__.py
    ├── saas_files_full.gql
    ├── saas_files_full.py
    ├── saas_files_small_with_provider.gql
    └── saas_files_small_with_provider.py

The generated .py files can be consumed like:

from gql_queries.saas_files import saas_files_full


with open("reconcile/gql_queries/saas_files/saas_files_full.gql", "r") as f:
    query = f.read()

data: dict[Any, Any] = gqlapi.query(query)
apps: list[AppV1] = saas_files_full.MyQuery(**data).apps_v1 or []

Other

There is a parallel PoC open for a 3rd party code generator #2367

@fishi0x01 fishi0x01 force-pushed the typed-gql-queries-2 branch 7 times, most recently from 294b25a to f612439 Compare May 11, 2022 09:21
@fishi0x01 fishi0x01 changed the title [WIP] 2nd PoC for typed gql queries design doc 2nd PoC for typed gql queries design doc May 11, 2022
@fishi0x01 fishi0x01 changed the title 2nd PoC for typed gql queries design doc 2nd PoC for Typed GQL Queries Design Doc May 11, 2022
@fishi0x01 fishi0x01 force-pushed the typed-gql-queries-2 branch 3 times, most recently from 8d3521b to 4e2c1e8 Compare May 11, 2022 10:52
@@ -76,6 +76,7 @@
],
entry_points={
'console_scripts': [
'query-generator = code_generator.main:main',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't this better fit in: https://github.com/app-sre/sretoolbox or even its own repository?
It does not directly relate to integrations.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fully agree. Would make sense to extract this.

If we finally decide to use a custom code generator, then I will move the code_generator module to a different repo or sretoolbox. Until then I will keep it here to have the whole PoC visible in a single PR.

@fishi0x01 fishi0x01 force-pushed the typed-gql-queries-2 branch 4 times, most recently from 8f1d395 to 93de0ca Compare May 16, 2022 08:23


def get_query_data(query_name: str) -> dict[Any, Any]:
gqlapi = gql.get_api()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Slightly of topic: can we try to get rid of using get_api if we refactor this? Having this singleton makes it hard to handle i.e. shared sessions to speed up queries.
I would love to see these as methods of a Gql Client class. Where we create instances of the Gql Client per thread.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point 👍

This is not really the intent of this PoC though. Focus is really on the code generation and mapping data to type aspect. I.e., we assume the nested untyped dict already exists somehow. I would like to keep this out of the discussion for the design doc, because the scope is already quite big as is. Including this topic would increase the scope even more.

If nobody cares how this client is implemented and there is no discussion needed, then Im fine refactoring it myself - as long as it is not part of this design doc

@fishi0x01 fishi0x01 force-pushed the typed-gql-queries-2 branch from 93de0ca to 19c8cf3 Compare June 2, 2022 14:24
@fishi0x01
Copy link
Contributor Author

This PoC is open since a while and there had been a schema change. The change was caught during static type checking phase -> as it should 😄 👍

@fishi0x01 fishi0x01 force-pushed the typed-gql-queries-2 branch from 93f163e to c1c2c0f Compare June 3, 2022 09:15
@fishi0x01 fishi0x01 force-pushed the typed-gql-queries-2 branch from 7ec9e51 to 3560872 Compare June 8, 2022 13:57
@fishi0x01
Copy link
Contributor Author

Code is migrated to dedicated repository https://github.com/fishi0x01/qenerate

Keeping this PR open until design doc is merged https://gitlab.cee.redhat.com/service/app-interface/-/merge_requests/41063

Will close this PR afterwards

@fishi0x01
Copy link
Contributor Author

Opened #2495 to use the new code generator on service_dependencies.

@fishi0x01 fishi0x01 closed this Jul 5, 2022
@fishi0x01 fishi0x01 deleted the typed-gql-queries-2 branch July 5, 2022 06:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants